Tweak URI-references and be smart with Strings #478
Conversation
|
I think this help address some of #456 too |
primer.md
Outdated
| Attributes of type URI-reference have quite a large set of valid values that | ||
| can be used. For example, it is possible that the `source` attribute could be | ||
| as short as `home` or could be more meaningful, such as | ||
| `com.example.my-component`. The CloudEvents specification allows for a wide |
There was a problem hiding this comment.
example.com/my-component sounds more like a proper URI-ref to me (unless my-component is a new TLD I haven't heard of yet 😉 )
primer.md
Outdated
| interoperability while still being flexible enough to be used in unique | ||
| situations. So, CloudEvent producers are encouraged to only use values that | ||
| have the best chance of being properly processed by all possible formats | ||
| and transports that their CloudEvents might be used with. |
There was a problem hiding this comment.
For SaaS publishers, their CloudEvents may end up being used with any format or transport. It'd be good to know the minimum one can rely on. ASCII? URL encoding?
There was a problem hiding this comment.
I thought about that too, but then I noticed that define Strings as Sequence of printable Unicode characters
which to me removed a fair about of the reasons for needing to URL encode stuff. Plus I didn't think we'd want to get into when, or when not, to encode Strings since then we'd need a mechanism by which consumers would have to know when they're encoded. That's when I decided "just don't be dumb" was good enough.
But if someone has any ideas...
primer.md
Outdated
| as short as `home` or could be more meaningful, such as | ||
| `http://home.example.com`. The CloudEvents specification allows for a wide | ||
| range of values to allow for it to be used in as any use cases as possible, | ||
| without undo burden on implementations. |
I can't think of a good reason why URI-references should be allowed to be blank since that seems about as useful as the property not being there at all. Signed-off-by: Doug Davis <dug@us.ibm.com>
primer.md
Outdated
| themselves based on their needs and expected usage of their events. | ||
|
|
||
| Additionally, the CloudEvents specification does not define a `baseURI` | ||
| to be used along with the URI-reference type attributes. This is because, |
There was a problem hiding this comment.
I wonder, if it prohibits that event producers would define a base URI in their documentation?
There was a problem hiding this comment.
I don't think we can stop anyone from doing so... but I'd question the need for it :-)
primer.md
Outdated
| unless otherwise stated, the URI-references are not meant to be normalized | ||
| or used for any other purpose other than comparing its string value against | ||
| some other URI-reference string value for equality. The use of URI-reference, | ||
| instead of just a String, was done to encourage the use of meaningful values |
There was a problem hiding this comment.
Referring to a complex specification such as RFC3986 just to encourage people to chose meaningful values while ignoring most of the specification, seems a bit strange.
There was a problem hiding this comment.
maybe but I think there is something to be said for encouraging some level of consistency, but still allowing a lot of freedom. Kind of like a best-practice. But, having said that, this is just my memory of our previous chats and if people think I'm remembering incorrectly, or they just don't want to even mention this aspect of it, I'd be ok with removing this sentence if that would help
|
up updated based on today's call. Let me know what you think |
|
Thank you @duglin! LGTM |
|
moved the "RECOMMENDED" line into the constraints section and deleted the rest of the paragraph - per today's call. Approved with the above change on the 8/15 call Can I get an LGTM to make sure I didn't mess anything up before I merge? |
spec.md
Outdated
| @@ -251,6 +251,8 @@ The following attributes are REQUIRED to be present in all CloudEvents: | |||
|
|
|||
| - Constraints: | |||
| - REQUIRED | |||
| - MUST be a non-empty URI-reference | |||
| - RECOMMENDED that an absolute URI be used | |||
There was a problem hiding this comment.
Disclaimer: I'm not a native English speaker. be used sounds strange to me here.
Either try is used or maybe An absolute URI is RECOMMENDED?
Signed-off-by: Doug Davis <dug@us.ibm.com>
|
thanks for the review @cneijenhuis merging. Approved on the 8/15 call |
I can't think of a good reason why URI-references should be allowed to
be blank since that seems about as useful as the property not being
there at all.
Closes #473
Signed-off-by: Doug Davis dug@us.ibm.com